Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

92 automatic profile creation for new user #111

Merged
merged 13 commits into from
Mar 7, 2017

Conversation

halfghaninne
Copy link
Collaborator

@halfghaninne halfghaninne commented Sep 7, 2016

Fixes #92


Login Credentials

Your GH profile. You should be able to create an "Admin" user to test this functionality.


Functionality to Test

  • When you create a new User, there is a form field where you can select the Cohort that User belongs to.
  • That form field will default either to the the next-upcoming class or -- if there is none -- to a message like "Please Select"
  • Upon User creation, a Profile is also created for them, and the User is linked to the appropriate Cohort through the Profile.

@halfghaninne
Copy link
Collaborator Author

halfghaninne commented Sep 7, 2016

This is a deep-dive into Administrate that I find intimidating and am not excited about at all. Can I put this on hold and we pair on this sometime early next week, @sumeetjain ?

Copy link
Owner

@sumeetjain sumeetjain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a question and some notes for things to improve.

@@ -3,6 +3,7 @@ source "https://rubygems.org"
ruby "2.3.0"

gem "administrate", "~> 0.2.2"
gem "administrate-field-nested_has_many", "~> 0.0.2"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need this gem?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't think so!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(upcoming.length > 0) ? upcoming[0].id : nil
end

def self.upcoming
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check out ActiveRecord scopes. This functionality would be better as a scope inside this model than it is as a class method.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -1,4 +1,13 @@
class Cohort < ActiveRecord::Base
has_many :profiles
has_many :users, through: :profiles

def self.next
(upcoming.length > 0) ? upcoming[0].id : nil
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Returning the ID is perhaps what you needed for this particular task, but for the purposes of building the API for Cohort, it'd be more useful (and no less computational effort for Ruby) to return the entire Cohort object.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@halfghaninne
Copy link
Collaborator Author

Made the changes you requested, though not sure how to dismiss the "Changes requested" message.

@sumeetjain sumeetjain merged commit ed2b747 into master Mar 7, 2017
@sumeetjain sumeetjain deleted the 92-automatic-profile-creation-for-new-user branch March 7, 2017 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants